Skip to content

[Testing] Fix wrong include in custom bootstrap for tests #11899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 30, 2019
Merged

[Testing] Fix wrong include in custom bootstrap for tests #11899

merged 1 commit into from
Jul 30, 2019

Conversation

gregurco
Copy link
Contributor

@gregurco gregurco commented Jul 5, 2019 •
edited
Loading

Hello.

There is a wrong include in custom tests/bootstrap.php for tests. Till 4.3 it was ok, but from 4.3 following the article you will receive next error:

LogicException: You must set the KERNEL_CLASS environment variable to the fully-qualified class name of your Kernel in phpunit.xml / phpunit.xml.dist or override the App\Tests\Foo\BarTest::createKernel() or App\Tests\Foo\BaTest::getKernelClass() method.

The problem exists, because the sequence of bootstrap is changes:
phpunit.dist.xml -> config/bootstrap.php -> vendor/autoload.php
and normally if something is injected in phpunit config, it should respect the sequence:
phpunit.dist.xml -> tests/bootstrap.php (CUSTOM) -> config/bootstrap.php -> vendor/autoload.php
Actual state in documentation (wrong one)
phpunit.dist.xml -> tests/bootstrap.php -> vendor/autoload.php

Red to recipe: https://github.com/symfony/recipes/blob/master/phpunit/phpunit/4.7/phpunit.xml.dist#L8

Thanks, Vlad.

@gregurco gregurco changed the title [Testing Fix wrong include in custom bootstrap for tests [Testing] Fix wrong include in custom bootstrap for tests Jul 5, 2019
@javiereguiluz
Copy link
Member

I think we should merge this in 3.4, because config/bootstrap.php was created in 3.3. See https://github.com/symfony/recipes/tree/master/symfony/framework-bundle/3.3/config @OskarStark @xabbuh what do you think? Thanks.

@xabbuh
Copy link
Member

xabbuh commented Jul 8, 2019

That's tricky because our setup guidelines don't make use of Flex so you won't get this file without doing so explicitly, right?

@gregurco
Copy link
Contributor Author

gregurco commented Jul 8, 2019

@xabbuh yes, I use Flex and it's new project.

@gregurco
Copy link
Contributor Author

@xabbuh do you have any questions? Is it ready to be merged or I have to change something?

@xabbuh
Copy link
Member

xabbuh commented Jul 15, 2019

Well, I still think that this does not really fit the 3.4 docs if people following the setup guide not using Flex will run into errors when following this change (because they do not have the bootstrap.php file).

@gregurco
Copy link
Contributor Author

@xabbuh I did a PR in 4.3, not in 3.4.

@xabbuh
Copy link
Member

xabbuh commented Jul 15, 2019

Yes, but Javier asked whether we should merge into the 3.4 branch.

@gregurco
Copy link
Contributor Author

I think it's not related to 3.4. I also started project with 3.4 and I didn't remember such a problem there.

@javiereguiluz javiereguiluz added this to the 4.2 milestone Jul 16, 2019
@javiereguiluz
Copy link
Member

If 3.4 is OK, then we'll merge in 4.2 Thanks.

@gregurco
Copy link
Contributor Author

@javiereguiluz right now I created new Symfony 3.4 project and in phpunit.xml.dist I found:

bootstrap="vendor/autoload.php"

So, it's not relevant for SF 3.4. Changes can be merged.

@gregurco
Copy link
Contributor Author

ping @javiereguiluz
could you please merge this PR?

@xabbuh xabbuh modified the milestones: 4.2, 4.3 Jul 28, 2019
@OskarStark
Copy link
Contributor

Thank you @gregurco.

@OskarStark OskarStark merged commit f34bd0b into symfony:4.3 Jul 30, 2019
OskarStark added a commit that referenced this pull request Jul 30, 2019
@OskarStark
minor #11899 [Testing] Fix wrong include in custom bootstrap for test…
08526cf 
…s (gregurco)

This PR was merged into the 4.3 branch.

Discussion
----------

[Testing] Fix wrong include in custom bootstrap for tests

Hello.

There is a wrong include in custom `tests/bootstrap.php` for tests. Till 4.3 it was ok, but from 4.3 following the article you will receive next error:
```
LogicException: You must set the KERNEL_CLASS environment variable to the fully-qualified class name of your Kernel in phpunit.xml / phpunit.xml.dist or override the App\Tests\Foo\BarTest::createKernel() or App\Tests\Foo\BaTest::getKernelClass() method.
```
The problem exists, because the sequence of bootstrap is changes:
`phpunit.dist.xml -> config/bootstrap.php -> vendor/autoload.php`
and normally if something is injected in phpunit config, it should respect the sequence:
`phpunit.dist.xml ->  tests/bootstrap.php (CUSTOM) -> config/bootstrap.php -> vendor/autoload.php`
Actual state in documentation (wrong one)
`phpunit.dist.xml ->  tests/bootstrap.php -> vendor/autoload.php`

Red to recipe: https://github.com/symfony/recipes/blob/master/phpunit/phpunit/4.7/phpunit.xml.dist#L8

Thanks, Vlad.

Commits
-------

f34bd0b Fix wrong include in custom bootstrap for tests
@gregurco gregurco deleted the patch-1 branch July 30, 2019 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xabbuh xabbuh xabbuh approved these changes

@javiereguiluz javiereguiluz Awaiting requested review from javiereguiluz

Assignees
No one assigned
Labels
Status: Reviewed Testing
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
5 participants
@gregurco @javiereguiluz @xabbuh @OskarStark @carsonbot